Skip to content

Allow passing expr metavariable to cfg #146961

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow passing expr metavariable to cfg #146961

wants to merge 1 commit into from

Conversation

Jules-Bertholet
Copy link
Contributor

@Jules-Bertholet Jules-Bertholet commented Sep 24, 2025 •
edited
Loading

This PR allows expanding expr metavariables inside the configuration predicates of cfg and cfg_attr invocations.
For example, the following code will now compile:

macro_rules! mac {
    ($e:expr) => {
        #[cfg_attr($e, inline)]
        #[cfg($e)]
        fn func() {}

        #[cfg(not($e))]
        fn func() {
            panic!()
        }
    }
}


mac!(any(unix, feature = "foo"));

There is currently no macro_rules fragment specifier that can represent all valid cfg predicates. meta comes closest, but excludes true and false. By fixing that, this change makes it easier to write declarative macros that parse cfg or cfg_attr invocations, for example #146281.

@rustbot label T-lang needs-fcp A-attributes A-cfg A-macros

@rustbot
Copy link
Collaborator

rustbot commented Sep 24, 2025

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 24, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-cfg Area: `cfg` conditional compilation A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. T-lang Relevant to the language team labels Sep 24, 2025
@Jules-Bertholet Jules-Bertholet mentioned this pull request Sep 24, 2025
Open
@fmease fmease added I-lang-nominated Nominated for discussion during a lang team meeting. S-waiting-on-team DEPRECATED: Use the team-based variants `S-waiting-on-t-lang`, `S-waiting-on-t-compiler`, ... and removed T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 24, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
RalfJung
RalfJung reviewed Sep 24, 2025
View reviewed changes
tests/ui/attributes/nonterminal-expansion.rs
}

pass_nonterminal!(n!());
//~^ ERROR expected one of `(`, `::`, or `=`, found `!`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error is surprisingly different from what you get when "inlining" the macro?

error: expected unsuffixed literal, found `!`
 --> src/main.rs:6:15
  |
6 | #[repr(align(n!()))]
  |               ^

https://play.rust-lang.org/?version=stable&mode=debug&edition=2024&gist=979f349ee4be2e88b300d4f97f5e1db9

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$expr effectively has parentheses around it, so the "inlined" version would look more like #[repr(align((n!())))].

@petrochenkov petrochenkov self-assigned this Sep 24, 2025
@traviscross traviscross added the P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang label Sep 24, 2025
@petrochenkov
Copy link
Contributor

petrochenkov commented Sep 26, 2025 •
edited
Loading

Just as an update, after #124141 we effectively no longer have nonterminal tokens in the grammar (*).
At parsing time a pasted expr metavariable is not some NtExpr token, it's just a token stream in invisible parentheses.
If that token stream from expr parses as a cfg predicate then it can be used as a cfg predicate, if it parses as a type or pattern, then it can be used as a pattern as well.
And on the other hand, if a token stream from some other matcher like ty or pat parses as a cfg predicate, then it can be used as a cfg predicate.
In general, the matcher kind is only relevant during macro LHS matching, not during RHS parsing (**).

I hoped that some time after #124141 either @nnethercote or me relax these rules in many cases, when there are no backward compatibility concerns at least, and pass the change through lang team, but it didn't happen yet.
(In particular, tokens from any matchers should be parse-able in expr, ty and pat positions, there should be no compatibility issues there.)

A number of current hacks like accepting impl $ty for Type, or what this PR suggests, are just special cases of that strategy.

@Jules-Bertholet You could very well start the same process, just from a different point and accept any token streams that look like cfg predicates as cfg predicates, from any matchers, not just expr.

(*) Or we technically have, but only for compatibility and to avoid extending the language without the lang team process.
(**) Again, unless there are backward compatibility issues.

@petrochenkov
Copy link
Contributor

As for the current implementation, the new expr-accepting logic is used in parse_meta_item and the new attr parsing infra, so it seems more than just cfg predicates specifically, but I'm not sure what else exactly is affected.

@petrochenkov petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 26, 2025
@Urgau Urgau added S-waiting-on-t-lang Status: Awaiting decision from T-lang and removed S-waiting-on-team DEPRECATED: Use the team-based variants `S-waiting-on-t-lang`, `S-waiting-on-t-compiler`, ... labels Oct 6, 2025
@joshtriplett
Copy link
Member

This seems reasonable to me. It allows all :expr fragments here, and not all of those will subsequently parse, but that seems okay.

@rfcbot merge

@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Oct 8, 2025 •
edited by traviscross
Loading

Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Oct 8, 2025
@traviscross
Copy link
Contributor

@rfcbot reviewed

@tmandry
Copy link
Member

tmandry commented Oct 8, 2025

@rfcbot reviewed

@rust-rfcbot rust-rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Oct 8, 2025
@rust-rfcbot
Copy link
Collaborator

🔔 This is now entering its final comment period, as per the review above. 🔔

@rust-rfcbot rust-rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Oct 8, 2025
@joshtriplett
Copy link
Member

As for the current implementation, the new expr-accepting logic is used in parse_meta_item and the new attr parsing infra, so it seems more than just cfg predicates specifically, but I'm not sure what else exactly is affected.

Happy to see a more general version of this, if we can.

@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

I am in favor of making the "special tokens" and invisible delimiters less visible to the user.

@traviscross traviscross added I-lang-radar Items that are on lang's radar and will need eventual work or consideration. and removed I-lang-nominated Nominated for discussion during a lang team meeting. P-lang-drag-1 Lang team prioritization drag level 1. https://rust-lang.zulipchat.com/#narrow/channel/410516-t-lang labels Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RalfJung RalfJung RalfJung left review comments

@petrochenkov petrochenkov petrochenkov left review comments

Assignees

@petrochenkov petrochenkov

@fee1-dead fee1-dead

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) A-cfg Area: `cfg` conditional compilation A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. needs-fcp This change is insta-stable, or significant enough to need a team FCP to proceed. S-waiting-on-t-lang Status: Awaiting decision from T-lang T-lang Relevant to the language team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.